Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v5] System for all actor logic #4055

Merged
merged 5 commits into from
Jun 8, 2023
Merged

[v5] System for all actor logic #4055

merged 5 commits into from
Jun 8, 2023

Conversation

davidkpiano
Copy link
Member

This PR provides access to system for all actor logic.

fromPromise(({ system }) => { ... });

fromTransition((state, event, { system }) => { ... });

fromObservable(({ system }) => { ... });

fromEventObservable(({ system }) => { ... });

fromCallback((sendBack, receive, { system }) => { ... });

@changeset-bot
Copy link

changeset-bot bot commented Jun 3, 2023

🦋 Changeset detected

Latest commit: 9268ef1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
xstate Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 3, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 9268ef1:

Sandbox Source
XState Example Template Configuration
XState React Template Configuration

@ghost
Copy link

ghost commented Jun 3, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@@ -131,7 +146,12 @@ export function fromObservable<T, TEvent extends EventObject>(
*/

export function fromEventObservable<T extends EventObject>(
lazyObservable: ({ input }: { input: any }) => Subscribable<T>
lazyObservable: ({
input
Copy link
Contributor

@mellson mellson Jun 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that input and/or system is defined as arguments differently across the fromFunctions. We only need the types right?

If that's the case wdyt about adding a shared interface for the types, something like this:

// Terrible name, please help 😅
interface FromRuntime {
  input: any;
  system: AnyActorSystem;
}

export function fromPromise<T>(
  // Leaving out the arguments as we don't use them
  promiseCreator: ({}: FromRuntime) => PromiseLike<T>
)...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do this in a follow-up PR, good idea

Copy link
Contributor

@mellson mellson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess my comment is just a type-level nit, so there is no reason why you shouldn't get an approval

@davidkpiano davidkpiano merged commit eb7c8b3 into next Jun 8, 2023
2 checks passed
@davidkpiano davidkpiano deleted the v5/system-for-actors branch June 8, 2023 23:55
@github-actions github-actions bot mentioned this pull request Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants